From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 12 Apr 2017 20:43:57 +0000 (-0700)
Subject: Ensure logo preload transforms urls if needed
X-Git-Tag: 1.31.0-rc.0~3520^2
X-Git-Url: http://git.cyclocoop.org/%7D%7Cconcat%7B?a=commitdiff_plain;h=45d8cd2926bb07ed1784f7dd10aa8b99d0c31971;p=lhc%2Fweb%2Fwiklou.git

Ensure logo preload transforms urls if needed

Follows-up 5f55e9c9c2a24.

If the logo url is from within /w, then ResourceLoaderSkinModule
will (as it should) apply a file hash query to it.

The preloader didn't do that, so it specified the wrong url.

Refactored SkinModule to make this logic re-usable.

Bug: T100999
Change-Id: I1ba11f7c70d1a725ad72754fee4a3f33c2a4c1be
---

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index df49bf6b88..5c05ad75d0 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -4005,29 +4005,22 @@ class OutputPage extends ContextSource {
 	 * @since 1.26
 	 */
 	protected function addLogoPreloadLinkHeaders() {
-		$logo = $this->getConfig()->get( 'Logo' ); // wgLogo
-		$logoHD = $this->getConfig()->get( 'LogoHD' ); // wgLogoHD
+		$logo = ResourceLoaderSkinModule::getLogo( $this->getConfig() );
 
 		$tags = [];
 		$logosPerDppx = [];
 		$logos = [];
 
-		$logosPerDppx['1.0'] = $logo;
-
-		if ( !$logoHD ) {
+		if ( !is_array( $logo ) ) {
 			// No media queries required if we only have one variant
 			$this->addLinkHeader( '<' . $logo . '>;rel=preload;as=image' );
 			return;
 		}
 
-		foreach ( $logoHD as $dppx => $src ) {
-			// Only 1.5x and 2x are supported
-			// Note: Keep in sync with ResourceLoaderSkinModule
-			if ( in_array( $dppx, [ '1.5x', '2x' ] ) ) {
-				// LogoHD uses a string in this format: "1.5x"
-				$dppx = substr( $dppx, 0, -1 );
-				$logosPerDppx[$dppx] = $src;
-			}
+		foreach ( $logo as $dppx => $src ) {
+			// Keys are in this format: "1.5x"
+			$dppx = substr( $dppx, 0, -1 );
+			$logosPerDppx[$dppx] = $src;
 		}
 
 		// Because PHP can't have floats as array keys
diff --git a/includes/resourceloader/ResourceLoaderSkinModule.php b/includes/resourceloader/ResourceLoaderSkinModule.php
index 7d37944f0e..5740925d2a 100644
--- a/includes/resourceloader/ResourceLoaderSkinModule.php
+++ b/includes/resourceloader/ResourceLoaderSkinModule.php
@@ -28,45 +28,77 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule {
 	 * @return array
 	 */
 	public function getStyles( ResourceLoaderContext $context ) {
-		$conf = $this->getConfig();
-		$logo = $conf->get( 'Logo' );
-		$logoHD = $conf->get( 'LogoHD' );
-
-		$logo1 = OutputPage::transformResourcePath( $conf, $logo );
-		$logo15 = OutputPage::transformResourcePath( $conf, $logoHD['1.5x'] );
-		$logo2 = OutputPage::transformResourcePath( $conf, $logoHD['2x'] );
-
+		$logo = $this->getLogo( $this->getConfig() );
 		$styles = parent::getStyles( $context );
+
+		$default = !is_array( $logo ) ? $logo : $logo['1x'];
 		$styles['all'][] = '.mw-wiki-logo { background-image: ' .
-			CSSMin::buildUrlValue( $logo1 ) .
-			'; }';
-		// Only 1.5x and 2x are supported
-		// Note: Keep in sync with OutputPage::addLogoPreloadLinkHeaders()
-		if ( $logoHD ) {
-			if ( isset( $logoHD['1.5x'] ) ) {
+				CSSMin::buildUrlValue( $default ) .
+				'; }';
+
+		if ( is_array( $logo ) ) {
+			if ( isset( $logo['1.5x'] ) ) {
 				$styles[
 					'(-webkit-min-device-pixel-ratio: 1.5), ' .
 					'(min--moz-device-pixel-ratio: 1.5), ' .
 					'(min-resolution: 1.5dppx), ' .
 					'(min-resolution: 144dpi)'
 				][] = '.mw-wiki-logo { background-image: ' .
-				CSSMin::buildUrlValue( $logo15 ) . ';' .
+				CSSMin::buildUrlValue( $logo['1.5x'] ) . ';' .
 				'background-size: 135px auto; }';
 			}
-			if ( isset( $logoHD['2x'] ) ) {
+			if ( isset( $logo['2x'] ) ) {
 				$styles[
 					'(-webkit-min-device-pixel-ratio: 2), ' .
 					'(min--moz-device-pixel-ratio: 2),' .
 					'(min-resolution: 2dppx), ' .
 					'(min-resolution: 192dpi)'
 				][] = '.mw-wiki-logo { background-image: ' .
-				CSSMin::buildUrlValue( $logo2 ) . ';' .
+				CSSMin::buildUrlValue( $logo['2x'] ) . ';' .
 				'background-size: 135px auto; }';
 			}
 		}
+
 		return $styles;
 	}
 
+	/**
+	 * @param Config $conf
+	 * @return string|array Single url if no variants are defined
+	 *  or array of logo urls keyed by dppx in form "<float>x".
+	 *  Key "1x" is always defined.
+	 */
+	public static function getLogo( Config $conf ) {
+		$logo = $conf->get( 'Logo' );
+		$logoHD = $conf->get( 'LogoHD' );
+
+		$logo1Url = OutputPage::transformResourcePath( $conf, $logo );
+
+		if ( !$logoHD ) {
+			return $logo1Url;
+		}
+
+		$logoUrls = [
+			'1x' => $logo1Url,
+		];
+
+		// Only 1.5x and 2x are supported
+		if ( isset( $logoHD['1.5x'] ) ) {
+			$logoUrls['1.5x'] = OutputPage::transformResourcePath(
+				$conf,
+				$logoHD['1.5x']
+			);
+		}
+		if ( isset( $logoHD['2x'] ) ) {
+			$logoUrls['2x'] = OutputPage::transformResourcePath(
+				$conf,
+				$logoHD['2x']
+			);
+		}
+
+		return $logoUrls;
+	}
+
 	/**
 	 * @param ResourceLoaderContext $context
 	 * @return bool
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 7571e261d4..9893f8c7e1 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -507,8 +507,12 @@ class OutputPageTest extends MediaWikiTestCase {
 	/**
 	 * @dataProvider providePreloadLinkHeaders
 	 * @covers OutputPage::addLogoPreloadLinkHeaders
+	 * @covers ResourceLoaderSkinModule::getLogo
 	 */
-	public function testPreloadLinkHeaders( $config, $result ) {
+	public function testPreloadLinkHeaders( $config, $result, $baseDir = null ) {
+		if ( $baseDir ) {
+			$this->setMwGlobals( 'IP', $baseDir );
+		}
 		$out = TestingAccessWrapper::newFromObject( $this->newInstance( $config ) );
 		$out->addLogoPreloadLinkHeaders();
 
@@ -519,6 +523,7 @@ class OutputPageTest extends MediaWikiTestCase {
 		return [
 			[
 				[
+					'ResourceBasePath' => '/w',
 					'Logo' => '/img/default.png',
 					'LogoHD' => [
 						'1.5x' => '/img/one-point-five.png',
@@ -533,6 +538,7 @@ class OutputPageTest extends MediaWikiTestCase {
 			],
 			[
 				[
+					'ResourceBasePath' => '/w',
 					'Logo' => '/img/default.png',
 					'LogoHD' => false,
 				],
@@ -540,6 +546,7 @@ class OutputPageTest extends MediaWikiTestCase {
 			],
 			[
 				[
+					'ResourceBasePath' => '/w',
 					'Logo' => '/img/default.png',
 					'LogoHD' => [
 						'2x' => '/img/two-x.png',
@@ -549,6 +556,16 @@ class OutputPageTest extends MediaWikiTestCase {
 				'not all and (min-resolution: 2dppx),' .
 				'</img/two-x.png>;rel=preload;as=image;media=(min-resolution: 2dppx)'
 			],
+			[
+				[
+					'ResourceBasePath' => '/w',
+					'Logo' => '/w/test.jpg',
+					'LogoHD' => false,
+					'UploadPath' => '/w/images',
+				],
+				'Link: </w/test.jpg?edcf2>;rel=preload;as=image',
+				'baseDir' => dirname( __DIR__ ) . '/data/media',
+			],
 		];
 	}